feat(automations): scheduled & manual agent tasks (core + web UI + CLI + agent tool)#100
feat(automations): scheduled & manual agent tasks (core + web UI + CLI + agent tool)#100cnjack wants to merge 6 commits into
Conversation
…I + agent tool) Add an Automations feature: run a jcode task on a schedule (hourly/daily/weekly) or manually. A run is just a normal session tagged with the automation id, so it reuses the existing Engine, transcript, diff and notifications. Core (internal/automation, leaf package, fully unit-tested incl. DST): - types/validate/schedule(pure ComputeNextRun)/templates - two-file store (automations.json + automation-state.json) with a cross-process flock write lock + atomic writes; volatile scheduler state kept apart from user definitions so frequent writes never clobber edits - single-owner scheduler elected via a separate flock; skip-if-running overlap guard, DST fall-back slot dedup, project-missing skip + auto-disable, a 30min liveness ceiling for scheduled runs, and a recover() guard so a panicking run never crashes the host process Web: - Runner reuses buildLocalEngine + submitMessage headlessly, captures the terminal error, and tears the throwaway engine down on completion (no idle-evict exists); scheduled runs forced to full_access - REST API: CRUD + run-now + runs + templates; PUT is a true partial patch - SessionMeta gains automation_id/trigger_kind/terminal_status/end_time/ error_reason; the main task list excludes automation runs; "Recent runs" filters them in with a Success/Failed status filter - Frontend: Automations overlay (list + recent runs + templates), New/Edit modal (cloud toggle greyed as "coming soon"), Pinia store, sidebar entry, i18n Agent tool (web/TUI/ACP): automation_create proposes an automation but always creates it DISABLED (human-in-the-loop) so a prompt-injected agent can't arm a recurring unattended run. CLI: jcode automation list/show/templates/enable/disable/delete. Design + edge-case analysis: docs/automations-prd.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds automations across the backend, web API, CLI, and frontend, including storage, scheduling, run execution, and UI navigation. Also introduces channels login/polling UI and standalone design mocks for the revised shell. ChangesAutomations and Shell Navigation
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant AutomationsView
participant API
participant Scheduler
participant Server.runAutomation
User->>Sidebar: open Automations
Sidebar->>AutomationsView: switch activeView
User->>AutomationsView: create / run now
AutomationsView->>API: POST /api/automations or /run
API->>Scheduler: update store / start async run
Scheduler->>Server.runAutomation: AutomationRunner.StartRun
Server.runAutomation-->>API: session metadata + terminal state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)internal/web/engine.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.16][ERROR]: unable to find a config; path internal/command/web.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.22][ERROR]: unable to find a config; path internal/web/server.go┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.14][ERROR]: unable to find a config; path
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cnjack
left a comment
There was a problem hiding this comment.
Staff Engineer Review — PR #100 feat(automations)
Reviewed all 36 changed files plus surrounding context (engine.go, session.go). The scheduler architecture, flock election, DST dedup, atomic writes, and panic recovery are solid. The bugs are concentrated in the glue layer. There is one critical, two high, and seven medium/low defects that need fixing before merge.
🔴 CRITICAL — Finding 1: Double registerEngine per automation run → engine pool exhausted after 64 runs
Files: internal/web/automation_run.go:62–101, internal/web/engine.go:340
buildLocalEngine("", ...) calls registerEngine(eng) internally (engine.go:340), storing the engine in s.tasks under its initial taskID (the recorder UUID from the factory). Back in runAutomation, eng.taskID = sid is then mutated to a different UUID, and registerEngine(eng) is called a second time — inserting the same engine under sid.
Consequences:
- Engine lives in
s.tasksunder two keys simultaneously. - Two
startPumpgoroutines run on the same event channel (first goroutine's cancel context is overwritten). deleteEngine(sid)removes onlysid; the original key leaks forever.- After 64 automation runs
len(s.tasks) >= maxLiveEngines→errTooManyTasks→ all new tasks and automation runs rejected by the server.
Fix: Pass sid into buildLocalEngine so there is exactly one registration:
rec, _ := session.NewRecorder(a.ProjectPath, prov, mdl)
sid := rec.UUID()
eng, err := s.buildLocalEngine(sid, a.ProjectPath, mode)
// remove the subsequent eng.taskID = sid and second registerEngine call🔴 HIGH — Finding 2: automation_create tool opens a fresh Store — automations are invisible to the server and scheduler
File: internal/tools/automation_tool.go:72
store, err := automation.NewStore() // throwaway instance, separate in-memory cacheThe server holds s.automations *automation.Store. Store.List() / Get() serve from in-memory s.defs, which is only refreshed inside withLock → loadLocked — and that only runs on writes. An automation created via the agent tool writes to disk through the throwaway store. The server's cache never learns about it until the next write cycle. Result:
GET /api/automationsreturns stale data; the user never sees the proposed automation.- The scheduler never fires the new automation.
Fix: Inject the server's live *automation.Store into tools.Env and use it in the tool instead of calling automation.NewStore().
🔴 HIGH — Finding 3: Manual "Run Now" has no concurrency guard
File: internal/web/automation_api.go:175-183
runAutomationAsync fires without consulting s.inflight or checking LastStatus. A double-click, a run_now=true racing a scheduled fire, or two clients simultaneously produce parallel agent sessions modifying the same project directory. The scheduler prevents scheduled↔scheduled overlap via s.inflight; the manual path bypasses it entirely.
Fix: Before firing, check s.automations.State(a.ID).LastStatus == automation.StatusRunning and return 409 Conflict. Alternatively, expose a SetInflight method on Scheduler and call it from runAutomationAsync.
🟡 MEDIUM — Finding 4: ConsecutiveFails not reset on SetEnabled(true) → permanent re-disable loop
File: internal/automation/store.go:265
After 5 consecutive skips, an automation auto-disables. The user fixes the problem and manually re-enables it — but ConsecutiveFails is still ≥ 5. The next single skip immediately re-disables again. The user cannot keep a recovered automation running without manually editing automation-state.json.
Fix:
func (s *Store) SetEnabled(id string, enabled bool) (*Automation, error) {
if enabled {
_ = s.UpdateState(id, func(rs *RunState) { rs.ConsecutiveFails = 0 })
}
return s.Update(id, func(a *Automation) { a.Enabled = enabled })
}🟡 MEDIUM — Finding 5: TOCTOU between UpdateState (sets disable=true) and SetEnabled(false)
File: internal/automation/scheduler.go:230-250
The disable flag is decided inside UpdateState's closure (lock held), but SetEnabled(false) is called outside the lock. A concurrent successful manual run can reset ConsecutiveFails = 0 in that window, causing SetEnabled(false) to incorrectly disable an automation that just succeeded.
Fix: Make the disable atomic — combine the defs update into the same withLock(true, true, ...) scope, or pass a disableOnThreshold flag into a single lock scope.
🟡 MEDIUM — Finding 6: Interrupted manual runs permanently stuck at StatusRunning
File: internal/automation/scheduler.go:115
reconcileStale() filters on TriggerSchedule only. A manual run active when the server crashes leaves LastStatus = running in automation-state.json forever — no recovery path short of manual file editing.
Fix: Extend reconciliation to all trigger types. For manual runs, use a time-based heuristic: if LastRunAt is more than ~2 hours old and status is still "running", reset to "interrupted".
🟡 MEDIUM — Finding 7: handleUpdateAutomation returns 400 for a not-found ID (should be 404)
File: internal/web/automation_api.go:140-142
Store.Update returns "automation %q not found" for a missing ID. The handler surfaces it as 400 Bad Request. Clients cannot distinguish validation failures from missing resources without parsing the error string.
Fix: Add a sentinel var ErrNotFound = errors.New("not found") in the store and check errors.Is(err, automation.ErrNotFound) in the handler to write 404.
🟡 LOW-MEDIUM — Finding 8: Data race on s.ctx in runAutomationAsync
s.ctx is written in Start() without a mutex and read in runAutomationAsync without a mutex. Benign in practice, but flagged by -race.
Fix: Use atomic.Pointer[context.Context] for s.ctx, or pass ctx as a parameter into runAutomationAsync.
🟡 LOW — Finding 9: ProjectPath accepts relative paths
File: internal/automation/validate.go:49-58
No filepath.IsAbs check. An automation with "." or "../other" fires against the server process's cwd, not the user's intended project.
Fix: if !filepath.IsAbs(p) { return fmt.Errorf("project_path must be an absolute path") }
🟡 LOW — Finding 10: /api/automations/runs O(total-sessions) scan with no pagination
File: internal/web/automation_api.go:203-234
Every request reads and parses the full session index and linearly scans all sessions across all projects. No limit/cursor. Latency grows as history accumulates.
Fix: Add ?limit=50&before=<timestamp> parameters. Long-term, maintain a dedicated automation-runs index rather than scanning all sessions.
Summary
| Priority | Finding | Severity |
|---|---|---|
| 1 | Double registerEngine → engine pool exhausts after 64 runs |
Critical |
| 2 | Agent tool creates separate Store → automations invisible to server |
High |
| 3 | No concurrent-run guard for manual runs | High |
| 4 | ConsecutiveFails not reset on re-enable → permanent disable loop |
Medium |
| 5 | TOCTOU on auto-disable flag | Medium |
| 6 | Manual run zombies never recovered on restart | Medium |
| 7 | handleUpdateAutomation returns 400 for not-found |
Medium |
| 8 | s.ctx data race |
Low-Medium |
| 9 | Relative paths accepted for ProjectPath |
Low |
| 10 | O(N) scan on /api/automations/runs, no pagination |
Low |
Finding 1 is a guaranteed production outage trigger — 64 scheduled fires exhaust the engine pool and block all subsequent tasks server-wide. Findings 2 and 3 break the primary agent-create UX flow and allow unsafe concurrent project mutations. Please fix at minimum Findings 1–3 before merge.
Generated by Claude Code
CI: check fmt.Fprint* return values in `jcode automation` commands (errcheck). Review findings (internal/automation, internal/web, internal/tools): - F1: runAutomation registered the engine twice (factory UUID + a freshly minted one), leaking a tasks-map entry per run and exhausting the engine pool after maxLiveEngines runs. Register once under eng.taskID and reuse the factory recorder (also unifies conversation + todo/goal snapshots). - F2: automation_create wrote through a throwaway Store, invisible to the server cache/API/scheduler. Inject the live Store via tools.Env. - F3: manual "Run Now" had no concurrency guard. Add a per-id in-flight claim (409), a scheduler skip on running state, and an authoritative atomic Store.TryMarkRunning claim in ExecuteRun (closes the scheduled-vs-manual and cross-process overlap window). - F4/F5: re-enabling cleared ConsecutiveFails only via SetEnabled; centralize the reset in Store.Update so the web PUT path gets it too, and fold the auto-disable into one lock scope (UpdateStateAndMaybeDisable) to close the TOCTOU. - F6: reconcileStale now recovers interrupted manual runs (stale/garbled LastRunAt), not just scheduled ones. - F7: ErrNotFound sentinel -> handleUpdateAutomation returns 404, not 400. - F8: Server context stored as atomic.Pointer (rootCtx) to fix the data race with the scheduler/manual-run goroutines. - F9: reject relative ProjectPath (filepath.IsAbs). - F10: paginate /api/automations/runs (?limit/?before). Adds regression tests for F1-F7 and F9. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
internal/automation/store.go (1)
388-398: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional: fsync before rename for crash durability.
writeJSONAtomicgives atomicity (reader sees old or new file, never partial), but without an fsync of the temp file and the parent directory, a crash right afteros.Renamecan still surface as a lost/emptyautomations.jsonon some filesystems. Acceptable for volatile state; worth considering for the user-edited definitions file.♻️ Durability hardening sketch
func writeJSONAtomic(path string, v any) error { b, err := json.MarshalIndent(v, "", " ") if err != nil { return err } tmp := path + ".tmp" - if err := os.WriteFile(tmp, b, 0o600); err != nil { - return err - } - return os.Rename(tmp, path) + f, err := os.OpenFile(tmp, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600) + if err != nil { + return err + } + if _, err := f.Write(b); err != nil { + _ = f.Close() + return err + } + if err := f.Sync(); err != nil { + _ = f.Close() + return err + } + if err := f.Close(); err != nil { + return err + } + return os.Rename(tmp, path) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/automation/store.go` around lines 388 - 398, The writeJSONAtomic function lacks explicit file synchronization (fsync) calls to ensure data is durably written to disk before the rename operation, which can lead to data loss on crash in some filesystems. After successfully writing the temp file with os.WriteFile, open the temp file, call Sync() on it to flush data to disk, and optionally sync the parent directory before performing the os.Rename operation to improve crash durability guarantees.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/automations-prd.md`:
- Around line 245-249: The documentation for the `POST
/api/automations/{id}/run` endpoint incorrectly states that it returns a
`session_id`, but the actual implementation returns a `202 Accepted` status with
a status payload and completes asynchronously. Update the documented response
for this endpoint in the HTTP API section to accurately reflect the actual
behavior: specify that it returns `202 Accepted` with a status payload rather
than a `session_id`, and clarify that the operation completes asynchronously
instead of returning immediate results.
- Around line 195-211: The PRD section on agent_create tool describes a blocking
request/resolve card flow with user confirmation before database writes, but the
actual implementation in internal/tools/automation_tool.go writes directly to
AutomationStore without any request blocking, and internal/web/automation_api.go
provides no request-id resolve endpoint. Update the PRD section to either
accurately reflect the current implementation (disabled-by-default mode with
direct writes), or explicitly mark the card machinery,
RequestAutomation/ResolveAutomation handlers, and blocking channel mechanism as
follow-up work items, ensuring implementers understand the actual security
contract in place rather than building against the described but unimplemented
flow.
- Around line 53-80: Add blank lines before and after each markdown table to
comply with markdownlint rules. Specifically, ensure there is a blank line
before the table header and after the closing row in all three tables (屏
A:自动化创建弹窗, 屏 B:自动化列表页, and 屏 C:模板页), as well as in the additional tables at
lines 233-242 and 285-297. Additionally, locate the fenced code block in section
§9.4 and add an appropriate language tag (such as yaml, json, python, etc.)
after the opening triple backticks to fix the missing language specification.
- Around line 91-95: The document contains conflicting specifications about lock
topology. Section D4 states the scheduler election lock is reused as the store
write lock, but sections §10.4 and others (also see lines 220-230, 290-292)
describe a two-lock topology where the election lock is separate from the store
write lock to avoid starving short writes. Decide on one final lock topology
(either reuse the election lock as store write lock per D4, or split into two
separate locks), then update all related sections including D4, D7, and the
problematic sections at 220-230 and 290-292 to consistently reflect that choice.
Ensure the store/scheduler contract clearly describes the chosen topology
throughout the document.
In `@internal/automation/store.go`:
- Around line 178-210: The List(), Get(), and State() methods serve stale data
because they only read from in-memory maps (defs and state) that are refreshed
exclusively when the current process performs writes via loadLocked(). In a
multi-process setup, changes made by other processes become invisible to the web
server. To fix this, add a loadLocked() call at the beginning of each read
method (List, Get, and State) while holding the mutex lock to refresh the cache
from persistent storage before reading and returning the data, ensuring these
methods always serve current information regardless of which process made recent
changes.
In `@internal/automation/validate.go`:
- Around line 101-103: The IsLocalPath function currently accepts both absolute
and relative paths by only checking if the path is non-empty and lacks a URL
scheme separator. Since ValidateAutomation expects absolute paths to prevent
unintended execution against the scheduler's working directory, modify
IsLocalPath to additionally validate that the path is absolute. Use the
filepath.IsAbs function from Go's standard library to enforce this check,
ensuring only absolute paths return true.
In `@internal/command/automation.go`:
- Around line 49-53: The automation command currently writes directly to stdout
using fmt.Println, fmt.Fprintln, and os.Stdout instead of routing through
Cobra's command writer methods. Replace all direct stdout writes with cmd.Print*
methods (cmd.Println for direct prints, cmd.PrintErrln for errors) and replace
os.Stdout references with cmd.OutOrStdout() when creating writers like
tabwriter.NewWriter. This applies throughout the file at the lines mentioned:
49-53, 60-61, 83-90, 100-104, 128, and 148, ensuring all output is properly
routed through Cobra for testability and control.
- Around line 45-46: Raw errors returned in the automation command are not
wrapped with context information. Locate all error return statements in the file
(at lines 45-46, 75-77, 121-127, and 142-147) and wrap each raw error with
fmt.Errorf using the pattern "automation: %w" to provide command-specific
context that preserves the failure origin while maintaining the underlying error
for debugging purposes.
In `@internal/tools/automation_tool.go`:
- Around line 67-70: After determining the `project` variable value (whether
from `in.ProjectPath` or `t.env.Pwd()`), resolve it through `env.ResolvePath()`
before passing it to `store.Create()`. This normalization step converts relative
paths to absolute paths and logs any path-escape warnings. Apply the resolved
path result back to the `project` variable so that the persisted value follows
the tool development guidelines for path handling.
In `@internal/web/automation_api.go`:
- Around line 256-288: The sort.Slice comparison function in the automation API
only sorts by StartTime, but since RFC3339 has second-precision, multiple runs
can share the same StartTime. Combined with random map iteration order in Go,
this causes non-deterministic ordering. Fix the sort.Slice comparison function
to include a stable tiebreaker when StartTime values are equal; add a secondary
comparison by SessionID (or another stable identifier) so that runs with
identical StartTime values maintain consistent relative ordering across
pagination requests using the before cursor.
In `@internal/web/server.go`:
- Around line 249-257: In the rootCtx() method of the Server struct, replace the
nil return statement with context.Background() to ensure a non-nil context is
always returned. This prevents panics in downstream callsites that pass
rootCtx() directly to context.WithTimeout and exec.CommandContext, which both
panic when receiving a nil context as their parent.
In `@web/src/components/AutomationEditorDialog.vue`:
- Around line 290-307: The CSS styles in this component contain hardcoded hex
color values instead of using CSS custom property tokens. In the .form-error
class, replace the hardcoded fallback color `#dc2626` with the --color-destructive
token. In the .btn-primary class, replace the hardcoded color `#fff` with the
--color-on-primary token. These changes ensure all colors derive from the theme
tokens defined in src/styles/tokens.css for consistent theming across the
application.
- Around line 106-109: The AutomationEditorDialog.vue component contains
hardcoded English strings for validation error messages (such as 'Name is
required.', 'Prompt is required.', and 'A project is required (no-project
automations cannot run unattended).') that are not localized. Replace all
hardcoded user-facing strings throughout the component, including the validation
error messages assigned to localError.value in the form validation checks and
any labels, actions, or hints in the template, with vue-i18n translation keys
using the $t() function to enable proper localization for non-English locales.
In `@web/src/components/AutomationsView.vue`:
- Around line 238-249: Replace all hardcoded hex colors with CSS custom property
tokens from src/styles/tokens.css. In the .ok class, replace `#16a34a` with the
appropriate success color token. In the .err class, replace `#dc2626` with the
corresponding danger or error color token variable instead of relying on the
fallback. In both the .run-btn and .btn-primary classes, replace the hardcoded
`#fff` color with --color-on-primary for text on primary background fills,
following the coding guidelines for color tokenization.
- Around line 80-170: The AutomationsView.vue component contains numerous
hardcoded strings throughout the template that prevent localization. Replace all
user-facing text strings with locale key references from the app's i18n system.
This includes: button labels like "New automation" and "Edit", headers like
"Your automations" and "Recent runs", empty state messages, filter options like
"All", "Success", and "Failed", placeholders, status labels, and descriptive
text like "Use agents to handle recurring work on a cadence you choose." and
"Start from a template — pick a project and confirm." Use the appropriate i18n
syntax (typically $t() in Vue) to reference the locale keys instead of embedding
strings directly in the template.
In `@web/src/stores/automation.ts`:
- Around line 60-62: The setEnabled function in automation.ts performs a
full-object update by spreading stripDerived(item) which can overwrite newer
fields if the item object is stale, causing lost updates. Modify the update call
within setEnabled to only pass the enabled property directly (as { enabled })
instead of spreading the entire stripDerived(item) object along with it,
ensuring only the toggle state is updated without affecting other fields.
---
Nitpick comments:
In `@internal/automation/store.go`:
- Around line 388-398: The writeJSONAtomic function lacks explicit file
synchronization (fsync) calls to ensure data is durably written to disk before
the rename operation, which can lead to data loss on crash in some filesystems.
After successfully writing the temp file with os.WriteFile, open the temp file,
call Sync() on it to flush data to disk, and optionally sync the parent
directory before performing the os.Rename operation to improve crash durability
guarantees.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25f8f726-76e2-4c01-a5ed-f975d42319ba
📒 Files selected for processing (43)
cmd/jcode/main.godocs/automations-prd.mdinternal/automation/filelock_unix.gointernal/automation/filelock_windows.gointernal/automation/schedule.gointernal/automation/schedule_test.gointernal/automation/scheduler.gointernal/automation/scheduler_test.gointernal/automation/store.gointernal/automation/store_test.gointernal/automation/templates.gointernal/automation/types.gointernal/automation/validate.gointernal/automation/validate_test.gointernal/command/acp.gointernal/command/automation.gointernal/command/interactive.gointernal/command/web.gointernal/session/session.gointernal/tools/automation_tool.gointernal/tools/automation_tool_test.gointernal/tools/env.gointernal/web/automation_api.gointernal/web/automation_api_test.gointernal/web/automation_run.gointernal/web/automation_run_test.gointernal/web/concurrency_test.gointernal/web/engine.gointernal/web/git_test.gointernal/web/server.gointernal/web/tasks_test.goweb/src/App.vueweb/src/components/AutomationEditorDialog.vueweb/src/components/AutomationsView.vueweb/src/components/Sidebar.vueweb/src/composables/api.tsweb/src/i18n/locales/en.tsweb/src/i18n/locales/ja.tsweb/src/i18n/locales/ko.tsweb/src/i18n/locales/zh-Hans.tsweb/src/i18n/locales/zh-Hant.tsweb/src/stores/automation.tsweb/src/types/automation.ts
| | 截图元素 | jcode 映射 | 备注 | | ||
| |---|---|---| | ||
| | Name | `Automation.Name` | — | | ||
| | Trigger(Daily 下拉) | `Trigger.Type=schedule` + `Cadence`(hourly/daily/weekly) | — | | ||
| | Hours / Minute | `Trigger.Hour/Minute`(weekly 再带 `Weekday`) | 本地时区 | | ||
| | Run in the cloud(开关) | `RunInCloud`(恒 false) | v1 不放死开关,改 tooltip「coming soon」 | | ||
| | Prompt(`Type / for skills`) | `Automation.Prompt` + `/` 唤起技能 | 复用 `GET /api/slash-commands` 补全 | | ||
| | Autopilot(左下) | `Automation.Mode`(Ask/Plan/Autopilot) | **schedule 触发强制 Autopilot**(见 §3、§7.4) | | ||
| | Select project | `Automation.ProjectPath` | **必填**;空 = 无人值守不支持 → skip+停用(§7.5) | | ||
| | Claude Sonnet 4.6 | `Provider`/`Model` | 留空=全局默认 | | ||
| | ~~High(推理力度)~~ | **去掉** | — | | ||
| | 「Without a project … quick chat」 | `ProjectPath==""` | jcode **不支持**无人值守跑(§7.5),与截图分歧 | | ||
| | Cancel / Create / Create and run | `POST /api/automations`(可带 `run_now`) | — | | ||
|
|
||
| ### 屏 B:自动化列表页 | ||
| | 截图元素 | jcode 映射 | | ||
| |---|---| | ||
| | 左侧导航「Automations」 | Sidebar 新增一级入口 | | ||
| | Tabs:All / Local / ~~Cloud~~ | v1 **砍掉 Cloud tab**(保留字段,留待云端) | | ||
| | Your automations 卡片(名/节奏徽标/prompt 预览/最近运行/▶) | `GET /api/automations`;▶ = `POST …/{id}/run` | | ||
| | Recent runs(按日期分组、状态、时间戳) | `AutomationID != ""` 的 session 子集 | | ||
| | 搜索框 | 前端过滤 | | ||
|
|
||
| ### 屏 C:模板页 | ||
| | 截图元素 | jcode 映射 | | ||
| |---|---| | ||
| | 6 张模板卡(带 Daily/Weekly/Manual 徽标) | 内置模板(embed),点卡→预填新建弹窗 | | ||
| | Skills 区「Turn an existing agent skill into an automation」 | 列 `GET /api/skills`,选一个→预填 `prompt=/<skill>` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clean up the markdownlint failures in the PRD.
The tables need surrounding blank lines, and the fenced code block in §9.4 needs a language tag. These are small but will keep the docs lint green.
Also applies to: 233-242, 285-297
🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: 您的意思是“"不"全”?
Context: .../ 唤起技能 | 复用 GET /api/slash-commands 补全 | | Autopilot(左下) | Automation.Mode(...
(BU)
🪛 markdownlint-cli2 (0.22.1)
[warning] 53-53: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 77-77: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/automations-prd.md` around lines 53 - 80, Add blank lines before and
after each markdown table to comply with markdownlint rules. Specifically,
ensure there is a blank line before the table header and after the closing row
in all three tables (屏 A:自动化创建弹窗, 屏 B:自动化列表页, and 屏 C:模板页), as well as in the
additional tables at lines 233-242 and 285-297. Additionally, locate the fenced
code block in section §9.4 and add an appropriate language tag (such as yaml,
json, python, etc.) after the opening triple backticks to fix the missing
language specification.
Source: Linters/SAST tools
| | D4 | 调度器 = **文件锁选主**(`~/.jcode/automation-scheduler.lock`);并把这把锁复用为存储写锁 | owner | | ||
| | D5 | `automation_create` 工具 = **human-in-the-loop**:走 `ask_user` 式阻塞回路,用户在卡片确认才落库 | owner | | ||
| | D6 | **运行时不加护栏**(无总开关/无单次上限/无强制审计) | owner | | ||
| | D7 | 存储 = **flock 写锁 + 易变调度态与用户定义分文件** | owner | | ||
| | D8 | 去掉 Effort | owner | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Pick one lock topology and keep it consistent.
§9.2/§9.3 say the scheduler election lock is reused as the store write lock, but §10.4 later says that reuse can starve short writes and should be split into two locks. The PRD needs one final choice here, because the store/scheduler contract changes materially depending on whether the election lock is shared or not.
Also applies to: 220-230, 290-292
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/automations-prd.md` around lines 91 - 95, The document contains
conflicting specifications about lock topology. Section D4 states the scheduler
election lock is reused as the store write lock, but sections §10.4 and others
(also see lines 220-230, 290-292) describe a two-lock topology where the
election lock is separate from the store write lock to avoid starving short
writes. Decide on one final lock topology (either reuse the election lock as
store write lock per D4, or split into two separate locks), then update all
related sections including D4, D7, and the problematic sections at 220-230 and
290-292 to consistently reflect that choice. Ensure the store/scheduler contract
clearly describes the chosen topology throughout the document.
| ## 8. agent_create 工具 + 渲染卡片(human-in-the-loop) | ||
|
|
||
| 让 agent 从自然语言创建自动化("以后每天早上帮我跑测试并总结失败项")。**安全闸门在创建处**:工具不直接落库,走 `ask_user` 式阻塞回路,用户在卡片上确认/编辑后才提交(D5)——挡住 prompt 注入静默造一条"每天自动批准"的后门。 | ||
|
|
||
| **机制**(照搬 `ask_user` 的 请求→卡片→resolve 阻塞回路,非 goal 那种被动 banner): | ||
| 1. agent 调 `automation_create`(参数=解析出的 name/prompt/trigger/project/mode)。 | ||
| 2. 工具 handler 调 `WebHandler.RequestAutomation(ctx, draft)` → emit `automation_request`(带唯一 id)→ **阻塞在 channel**(仿 `RequestAskUser`,`internal/handler/web.go:283-318/515-532`)。 | ||
| 3. WS → `AutomationCard.vue` 渲染草稿预览 + Confirm/Edit/Cancel。 | ||
| 4. 用户 Confirm → `POST /api/automations`(带 request id)→ `ResolveAutomation(id, draft')` → 经**唯一** `automation.Store.Create`+`ValidateAutomation` 落库 → 解开 channel,工具返回「已创建」。Cancel → 工具返回「用户取消」。 | ||
| 5. 弹窗路径与工具路径**共用同一个 `Store.Create`**(仿 goal 单校验),唯一差别是"谁点的 Create"。 | ||
|
|
||
| **新增/改动文件**(研究已勘定): | ||
| - 新增 `internal/tools/automation.go`(`automation_create` 工具 + 草稿类型)。注意:工具落库的是 `internal/automation.Store`,不在 tools 包重造存储——tools 包仅持一个对 Store 的引用(挂在 `tools.Env`,仿 `Env.GoalStore`,`internal/tools/env.go`)。 | ||
| - 注册:两处 `buildAllTools` 各加一行 —— `internal/command/web.go:286-313`、`internal/command/interactive.go:82-110`(全前端自动获得)。 | ||
| - `internal/handler/web.go`:加 `WebAutomationRequestData` + `RequestAutomation`/`ResolveAutomation`(镜像 ask_user)。 | ||
| - `internal/web/server.go`:`POST /api/automations` 兼作 resolve(带 request id 时)。 | ||
| - 前端:新增 `web/src/components/AutomationCard.vue`(仿 `AskUserCard.vue`);`ws.ts` 加 `automation_request` 派发;`stores/chat.ts` 加 `onAutomationRequest`/`submitAutomation`;`types/api.ts`、`api.ts` 加类型与端点。 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reconcile the human-in-the-loop flow with the shipped tool path.
This section still describes a blocking request/resolve card flow, but the current internal/tools/automation_tool.go writes directly through AutomationStore and internal/web/automation_api.go exposes no request-id resolve path. Please either update the PRD to the actual disabled-by-default flow or mark the card machinery as explicit follow-up work; otherwise implementers will build against the wrong security contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/automations-prd.md` around lines 195 - 211, The PRD section on
agent_create tool describes a blocking request/resolve card flow with user
confirmation before database writes, but the actual implementation in
internal/tools/automation_tool.go writes directly to AutomationStore without any
request blocking, and internal/web/automation_api.go provides no request-id
resolve endpoint. Update the PRD section to either accurately reflect the
current implementation (disabled-by-default mode with direct writes), or
explicitly mark the card machinery, RequestAutomation/ResolveAutomation
handlers, and blocking channel mechanism as follow-up work items, ensuring
implementers understand the actual security contract in place rather than
building against the described but unimplemented flow.
| ### 9.5 HTTP API(`internal/web/server.go` 新增) | ||
| - `GET/POST /api/automations`(POST 带 `run_now` = Create and run;带 request id = resolve agent 草稿) | ||
| - `GET/PUT/DELETE /api/automations/{id}` | ||
| - `POST /api/automations/{id}/run`(手动,返回 session_id) | ||
| - `GET /api/automations/runs[?automation_id=]`(= `ListAllSessions` 过滤) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fix the documented response for manual runs.
POST /api/automations/{id}/run is documented here as returning a session_id, but the current handler returns 202 Accepted with a status payload and completes asynchronously. Please align the PRD with the actual contract so clients don’t wait for a field that never appears.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/automations-prd.md` around lines 245 - 249, The documentation for the
`POST /api/automations/{id}/run` endpoint incorrectly states that it returns a
`session_id`, but the actual implementation returns a `202 Accepted` status with
a status payload and completes asynchronously. Update the documented response
for this endpoint in the HTTP API section to accurately reflect the actual
behavior: specify that it returns `202 Accepted` with a status payload rather
than a `session_id`, and clarify that the operation completes asynchronously
instead of returning immediate results.
| // List returns all automations sorted by creation time. | ||
| func (s *Store) List() []*Automation { | ||
| s.mu.RLock() | ||
| defer s.mu.RUnlock() | ||
| out := s.listLocked() | ||
| cp := make([]*Automation, len(out)) | ||
| for i, a := range out { | ||
| c := *a | ||
| cp[i] = &c | ||
| } | ||
| return cp | ||
| } | ||
|
|
||
| // Get returns a copy of the automation, or nil if not found. | ||
| func (s *Store) Get(id string) *Automation { | ||
| s.mu.RLock() | ||
| defer s.mu.RUnlock() | ||
| if a, ok := s.defs[id]; ok { | ||
| c := *a | ||
| return &c | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // State returns a copy of the run-state for an automation (zero value if none). | ||
| func (s *Store) State(id string) RunState { | ||
| s.mu.RLock() | ||
| defer s.mu.RUnlock() | ||
| if st, ok := s.state[id]; ok { | ||
| return *st | ||
| } | ||
| return RunState{} | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# How does the web layer obtain/refresh the automation Store for reads?
fd 'automation_api.go|server.go|engine.go' internal/web --exec cat -n {}
rg -nP '\bNewStore(Dir)?\b|\.List\(\)|\.Get\(|\.State\(|reload|loadLocked' internal/webRepository: cnjack/jcode
Length of output: 50369
Reads from the web API serve a stale in-memory cache.
List, Get, and State read only from the in-memory defs/state maps. Those maps are refreshed (loadLocked) exclusively inside withLock, i.e. only when this process performs a write. A definition or state change made by a different jcode process (CLI enable/disable/delete, TUI, or the scheduler) is therefore invisible to the web server's reads until the web server happens to perform its own write.
The web handlers (handleListAutomations at line 49, handleGetAutomation at line 62, handleRunAutomation at line 185) call st.List(), st.Get(), and st.State() directly without an intermediate refresh, so a long-lived web server can keep serving an out-of-date automation list/state after a CLI edit in the multi-process setup. Writes are correctly serialized; only the read path lacks a refresh.
Consider adding a loadLocked() call before serving these read endpoints, or implementing a periodic reload to keep the cache synchronized with changes from other processes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/automation/store.go` around lines 178 - 210, The List(), Get(), and
State() methods serve stale data because they only read from in-memory maps
(defs and state) that are refreshed exclusively when the current process
performs writes via loadLocked(). In a multi-process setup, changes made by
other processes become invisible to the web server. To fix this, add a
loadLocked() call at the beginning of each read method (List, Get, and State)
while holding the mutex lock to refresh the cache from persistent storage before
reading and returning the data, ensuring these methods always serve current
information regardless of which process made recent changes.
| if (!form.name.trim()) { localError.value = 'Name is required.'; return } | ||
| if (!form.prompt.trim()) { localError.value = 'Prompt is required.'; return } | ||
| if (!form.projectPath.trim()) { localError.value = 'A project is required (no-project automations cannot run unattended).'; return } | ||
| saving.value = true |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Localize the dialog copy and validation messages via vue-i18n.
This modal currently hardcodes user-facing English strings (labels, actions, hints, and validation errors), so the automations flow won’t be translated in non-English locales.
Also applies to: 119-119, 137-215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/AutomationEditorDialog.vue` around lines 106 - 109, The
AutomationEditorDialog.vue component contains hardcoded English strings for
validation error messages (such as 'Name is required.', 'Prompt is required.',
and 'A project is required (no-project automations cannot run unattended).')
that are not localized. Replace all hardcoded user-facing strings throughout the
component, including the validation error messages assigned to localError.value
in the form validation checks and any labels, actions, or hints in the template,
with vue-i18n translation keys using the $t() function to enable proper
localization for non-English locales.
| <h1>Automations</h1> | ||
| <div class="seg"> | ||
| <button :class="['seg-btn', { on: view === 'list' }]" @click="view = 'list'">Your automations</button> | ||
| <button :class="['seg-btn', { on: view === 'templates' }]" @click="view = 'templates'">Templates</button> | ||
| </div> | ||
| </div> | ||
| <div class="auto-top-right"> | ||
| <button class="btn-primary" @click="newAutomation">New automation</button> | ||
| <button class="icon-btn" aria-label="Close" @click="emit('close')"><XMarkIcon class="w-5 h-5" /></button> | ||
| </div> | ||
| </header> | ||
|
|
||
| <div class="auto-scroll"> | ||
| <!-- ── Your automations ── --> | ||
| <template v-if="view === 'list'"> | ||
| <p class="section-sub">Use agents to handle recurring work on a cadence you choose.</p> | ||
|
|
||
| <div v-if="store.loading && !store.items.length" class="empty">Loading…</div> | ||
| <div v-else-if="!store.items.length" class="empty"> | ||
| No automations yet. Click <strong>New automation</strong> or pick a | ||
| <button class="link" @click="view = 'templates'">template</button>. | ||
| </div> | ||
|
|
||
| <div v-else class="cards"> | ||
| <div v-for="a in store.items" :key="a.id" class="card" :class="{ disabled: !a.enabled }"> | ||
| <div class="card-head"> | ||
| <span class="card-name">{{ a.name }}</span> | ||
| <span class="badge">{{ a.badge }}</span> | ||
| </div> | ||
| <p class="card-prompt">{{ a.prompt }}</p> | ||
| <div class="card-foot"> | ||
| <span class="card-meta"> | ||
| <CheckCircleIcon v-if="a.state.last_status === 'success'" class="w-3.5 h-3.5 ok" /> | ||
| <ExclamationCircleIcon v-else-if="a.state.last_status === 'error'" class="w-3.5 h-3.5 err" /> | ||
| {{ a.human_schedule }}<template v-if="!a.enabled"> · paused</template> | ||
| </span> | ||
| <div class="card-actions"> | ||
| <button class="icon-btn sm" title="Edit" @click="editAutomation(a)"><PencilSquareIcon class="w-4 h-4" /></button> | ||
| <button class="icon-btn sm" title="Delete" @click="store.remove(a.id)"><TrashIcon class="w-4 h-4" /></button> | ||
| <label class="switch" :title="a.enabled ? 'Enabled' : 'Disabled'"> | ||
| <input type="checkbox" :checked="a.enabled" @change="store.setEnabled(a, ($event.target as HTMLInputElement).checked)" /> | ||
| <span class="switch-track"><span class="switch-knob" /></span> | ||
| </label> | ||
| <button class="run-btn" :disabled="isRunning(a)" title="Run now" @click="store.runNow(a.id)"> | ||
| <PlayIcon class="w-4 h-4" /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- ── Recent runs ── --> | ||
| <div class="runs-head"> | ||
| <h2>Recent runs</h2> | ||
| <div class="runs-tools"> | ||
| <select v-model="statusFilter" class="status-filter"> | ||
| <option value="all">All</option> | ||
| <option value="success">Success</option> | ||
| <option value="failed">Failed</option> | ||
| </select> | ||
| <input v-model="search" class="run-search" :placeholder="`Search ${store.runs.length} runs…`" /> | ||
| </div> | ||
| </div> | ||
| <div v-if="!filteredRuns.length" class="empty sm">No runs yet.</div> | ||
| <div v-else class="runs"> | ||
| <div v-for="r in filteredRuns" :key="r.session_id" class="run-row"> | ||
| <div class="run-main"> | ||
| <span class="run-title">{{ r.title || 'Automation run' }}</span> | ||
| <span class="run-time"> | ||
| <CheckCircleIcon v-if="r.terminal_status === 'success'" class="w-3.5 h-3.5 ok" /> | ||
| <ExclamationCircleIcon v-else-if="r.terminal_status === 'error'" class="w-3.5 h-3.5 err" /> | ||
| <span v-else class="dot" /> | ||
| {{ runLabel(r) }} · {{ r.trigger_kind }} | ||
| </span> | ||
| </div> | ||
| <span v-if="r.error_reason" class="run-err">{{ r.error_reason }}</span> | ||
| </div> | ||
| </div> | ||
| </template> | ||
|
|
||
| <!-- ── Templates ── --> | ||
| <template v-else> | ||
| <p class="section-sub">Start from a template — pick a project and confirm.</p> | ||
| <div class="tpl-grid"> | ||
| <button v-for="t in store.templates" :key="t.id" class="tpl-card" @click="fromTemplate(t)"> | ||
| <div class="card-head"> | ||
| <span class="card-name">{{ t.name }}</span> | ||
| <span class="badge">{{ t.badge }}</span> | ||
| </div> | ||
| <p class="card-prompt">{{ t.description }}</p> | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move AutomationsView copy to locale keys instead of hardcoded strings.
The new automations UI text is hardcoded in-template (headers, buttons, empty states, filters, and run labels), so this feature won’t fully localize across supported languages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/AutomationsView.vue` around lines 80 - 170, The
AutomationsView.vue component contains numerous hardcoded strings throughout the
template that prevent localization. Replace all user-facing text strings with
locale key references from the app's i18n system. This includes: button labels
like "New automation" and "Edit", headers like "Your automations" and "Recent
runs", empty state messages, filter options like "All", "Success", and "Failed",
placeholders, status labels, and descriptive text like "Use agents to handle
recurring work on a cadence you choose." and "Start from a template — pick a
project and confirm." Use the appropriate i18n syntax (typically $t() in Vue) to
reference the locale keys instead of embedding strings directly in the template.
| async function setEnabled(item: AutomationItem, enabled: boolean) { | ||
| await update(item.id, { ...stripDerived(item), enabled }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Avoid full-object updates when toggling enabled state.
Line 61 sends a broad snapshot (stripDerived(item)) for a simple enable/disable toggle. If item is stale, this can overwrite newer fields (for example prompt/mode) and cause lost updates. Send only { enabled } for this path.
Suggested fix
async function setEnabled(item: AutomationItem, enabled: boolean) {
- await update(item.id, { ...stripDerived(item), enabled })
+ await update(item.id, { enabled })
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/stores/automation.ts` around lines 60 - 62, The setEnabled function
in automation.ts performs a full-object update by spreading stripDerived(item)
which can overwrite newer fields if the item object is stale, causing lost
updates. Modify the update call within setEnabled to only pass the enabled
property directly (as { enabled }) instead of spreading the entire
stripDerived(item) object along with it, ensuring only the toggle state is
updated without affecting other fields.
The header (.auto-top) spanned the full window while the scroll body was capped at max-width:1100px and centered, so on wide screens the title and buttons didn't line up with the section text, cards, and Recent runs below. Give the header the same centered 1100px column so everything aligns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.auto-shell was position:fixed inset:0, covering the whole window — opening Automations hid the sidebar. Start the overlay at the sidebar's right edge (left: var(--sidebar-width)) so it covers only the main content area and the sidebar stays visible, matching how Settings/Projects don't take over the shell. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Redesign the sidebar header into a whitespace nav (C2: no lines, just air) with three equal controls — New task, Automations, Channels — each showing its shortcut. Add a Channels page (remote-control landing: promo card + phone mock + WeChat connect/QR flow) and a shared PageSurface component so the Automations and Channels pages share one inset-surface shell. - Sidebar.vue: whitespace nav rows replace the bordered buttons; ⚡ → Sparkles; Signal icon + openChannels emit; platform-aware kbd hints - ChannelsView.vue (new): promo landing, WeChat login/QR/poll (mirrors SettingsDialog's tab logic), phone mock - PageSurface.vue (new): shared inset chrome + title head for secondary pages (no close button — dismissal via Esc/nav/task-click) - AutomationsView/ChannelsView: adopt PageSurface; drop duplicated chrome + close buttons; style.css Tauri margin override → .page-surface - App.vue: activeView gains 'channels'; ⌘⇧A/⌘⇧C shortcuts; Esc returns from any non-chat page; ChannelsView mounted - Sidebar.vue openTask: goToChat() before loadSession so opening a task from a non-chat page returns to the canvas (was a no-op behind the page) - i18n: nav.channels + channels.* (promo/features/mock) in 5 locales - design/: exploration mocks (sidebar-redesign, nav-actions-redesign)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/src/components/MenuSelect.vue (1)
13-13: 🚀 Performance & Scalability | 🔵 TrivialUse per-icon Heroicons subpath imports for tree-shaking.
Line 13 uses a grouped import from the barrel; the guideline requires per-file subpath imports for tree-shaking. However, this import pattern is consistent across the entire
web/srccodebase—all components currently use grouped barrel imports rather than subpath imports. Refactoring this should be part of a broader migration, not just this file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/MenuSelect.vue` at line 13, The import statement for ChevronUpDownIcon and CheckIcon uses a grouped barrel import from '`@heroicons/vue/24/outline`' instead of per-file subpath imports required by the guidelines for tree-shaking optimization. Replace the grouped import statement with individual per-file subpath imports where each icon is imported from its own dedicated path (e.g., import ChevronUpDownIcon from '`@heroicons/vue/24/outline/ChevronUpDownIcon`' and import CheckIcon from '`@heroicons/vue/24/outline/CheckIcon`' or similar subpath pattern).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/components/ChannelsView.vue`:
- Around line 228-400: The CSS in the style block contains multiple hardcoded
hex color values that violate the coding guideline requiring all colors to come
from design tokens. Replace the hardcoded colors in the following locations: the
fallback `#fff` in the .btn-primary class color property, the `#07C160` and `#fff`
hardcoded values in the .wc-btn.approve class, and the `#07C160` used in the
.wc-pill class color and background color-mix function. Use appropriate design
tokens from tokens.css (such as --color-success for the WeChat green,
--color-on-primary for the white/light color, and similar tokens) to replace all
hardcoded hex values.
In `@web/src/components/Sidebar.vue`:
- Around line 426-458: The nav-ic class is being used to size the PlusIcon,
SparklesIcon, and SignalIcon elements in the sidebar header nav-row buttons,
which violates the coding guideline to use Tailwind utilities for icon sizing.
Replace the nav-ic class usage on each of these icon elements with appropriate
Tailwind sizing classes (such as w-5 h-5 or w-4 h-4 depending on the intended
size), then remove the width and height CSS rules from the nav-ic class
definition in the stylesheet to complete the migration to Tailwind utilities.
In `@web/src/i18n/locales/zh-Hant.ts`:
- Line 38: The Traditional Chinese translations for "channels" terminology are
inconsistent across the file. The entries at lines 38 and 66 use `通道`, but the
`settings.tabs.channels` key uses `頻道` for the same concept, which creates
confusion in navigation and settings paths for users. Identify all occurrences
where "channels" is translated in the zh-Hant.ts file (including the `channels`
key around line 38, the second occurrence around line 66, and the
`settings.tabs.channels` entry) and ensure they all use the same consistent
Chinese term throughout the file.
---
Nitpick comments:
In `@web/src/components/MenuSelect.vue`:
- Line 13: The import statement for ChevronUpDownIcon and CheckIcon uses a
grouped barrel import from '`@heroicons/vue/24/outline`' instead of per-file
subpath imports required by the guidelines for tree-shaking optimization.
Replace the grouped import statement with individual per-file subpath imports
where each icon is imported from its own dedicated path (e.g., import
ChevronUpDownIcon from '`@heroicons/vue/24/outline/ChevronUpDownIcon`' and import
CheckIcon from '`@heroicons/vue/24/outline/CheckIcon`' or similar subpath
pattern).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b566e986-3605-4273-928e-bcee38e9017c
📒 Files selected for processing (17)
design/nav-actions-redesign.htmldesign/sidebar-redesign.htmlweb/src/App.vueweb/src/components/AutomationEditorDialog.vueweb/src/components/AutomationsView.vueweb/src/components/ChannelsView.vueweb/src/components/MenuSelect.vueweb/src/components/PageSurface.vueweb/src/components/ProjectPickerPanel.vueweb/src/components/Sidebar.vueweb/src/components/TopBar.vueweb/src/i18n/locales/en.tsweb/src/i18n/locales/ja.tsweb/src/i18n/locales/ko.tsweb/src/i18n/locales/zh-Hans.tsweb/src/i18n/locales/zh-Hant.tsweb/src/style.css
✅ Files skipped from review due to trivial changes (4)
- web/src/i18n/locales/ja.ts
- web/src/i18n/locales/zh-Hans.ts
- design/sidebar-redesign.html
- web/src/i18n/locales/ko.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/components/AutomationEditorDialog.vue
| <style scoped> | ||
| /* The page surface (inset chrome + title head + scroll body) is owned by | ||
| * PageSurface. This component styles only its own content. Content column is | ||
| * centered + inset to match the chat timeline, scoped to PageSurface's body. */ | ||
| :deep(.page-body) > * { max-width: 56rem; margin-left: auto; margin-right: auto; padding: 0 20px 32px; } | ||
|
|
||
| .chan-stage { display: flex; gap: 40px; align-items: flex-start; padding-top: 16px; } | ||
| @media (max-width: 860px) { .chan-stage { flex-direction: column; } } | ||
|
|
||
| /* ── Promo card (left-anchored) ── */ | ||
| .promo-card { | ||
| flex: 1; | ||
| min-width: 0; | ||
| position: relative; | ||
| background: | ||
| radial-gradient(120% 80% at 0% 0%, color-mix(in srgb, var(--color-primary) 8%, transparent), transparent 60%), | ||
| var(--color-background); | ||
| border: 1px solid var(--color-border); | ||
| border-radius: var(--radius-2xl); | ||
| overflow: hidden; | ||
| } | ||
| .promo-glow { | ||
| position: absolute; | ||
| top: -90px; left: -60px; | ||
| width: 280px; height: 280px; | ||
| border-radius: 50%; | ||
| background: color-mix(in srgb, var(--color-primary) 13%, transparent); | ||
| filter: blur(46px); | ||
| opacity: 0.8; | ||
| pointer-events: none; | ||
| } | ||
| .promo-inner { position: relative; padding: 32px; display: flex; flex-direction: column; } | ||
| .promo-eyebrow { | ||
| align-self: flex-start; | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 7px; | ||
| font-size: 11px; | ||
| font-weight: 600; | ||
| letter-spacing: 0.04em; | ||
| text-transform: uppercase; | ||
| color: var(--color-primary); | ||
| padding: 5px 11px; | ||
| border-radius: var(--radius-pill); | ||
| background: color-mix(in srgb, var(--color-primary) 8%, transparent); | ||
| border: 1px solid color-mix(in srgb, var(--color-primary) 35%, transparent); | ||
| } | ||
| .promo-title { | ||
| font-size: 26px; | ||
| font-weight: 600; | ||
| letter-spacing: -0.02em; | ||
| margin: 18px 0 0; | ||
| line-height: 1.15; | ||
| } | ||
| .promo-title :deep(.accent) { color: var(--color-primary); } | ||
| .promo-lede { | ||
| font-size: 13.5px; | ||
| color: var(--color-muted-foreground); | ||
| line-height: 1.6; | ||
| margin: 14px 0 0; | ||
| max-width: 44ch; | ||
| } | ||
|
|
||
| .promo-features { display: flex; flex-direction: column; gap: 16px; margin: 24px 0 0; } | ||
| .promo-feat { display: flex; gap: 13px; align-items: flex-start; } | ||
| .feat-ic { | ||
| display: grid; place-items: center; | ||
| width: 36px; height: 36px; flex-shrink: 0; | ||
| border-radius: var(--radius-lg); | ||
| background: color-mix(in srgb, var(--color-foreground) 8%, transparent); | ||
| border: 1px solid color-mix(in srgb, var(--color-foreground) 30%, transparent); | ||
| color: var(--color-foreground); | ||
| } | ||
| .promo-feat h4 { font-size: 13.5px; font-weight: 600; margin: 3px 0 3px; } | ||
| .promo-feat p { font-size: 12.5px; color: var(--color-muted-foreground); line-height: 1.5; margin: 0; max-width: 42ch; } | ||
|
|
||
| .promo-cta { display: flex; align-items: center; gap: 14px; margin-top: 26px; flex-wrap: wrap; } | ||
| .btn-primary { | ||
| display: inline-flex; align-items: center; gap: 8px; | ||
| padding: 10px 18px; | ||
| background: var(--color-primary); | ||
| color: var(--color-on-primary, #fff); | ||
| border: none; | ||
| border-radius: var(--radius-pill); | ||
| font-size: 13px; | ||
| font-weight: 600; | ||
| cursor: pointer; | ||
| box-shadow: 0 4px 16px -4px color-mix(in srgb, var(--color-primary) 60%, transparent); | ||
| transition: box-shadow 0.15s, opacity 0.15s; | ||
| } | ||
| .btn-primary:hover:not(:disabled) { box-shadow: 0 6px 22px -5px color-mix(in srgb, var(--color-primary) 70%, transparent); } | ||
| .btn-primary:disabled { opacity: 0.6; cursor: not-allowed; } | ||
| .cta-hint { font-size: 11.5px; color: var(--color-muted-foreground); } | ||
|
|
||
| /* ── Connect (QR) ── */ | ||
| .qr-block { display: flex; flex-direction: column; align-items: center; gap: 10px; margin-top: 22px; } | ||
| .qr-canvas { border: 1px solid var(--color-border); border-radius: var(--radius-md); } | ||
| .qr-hint { font-size: 12px; color: var(--color-muted-foreground); } | ||
|
|
||
| /* ── Connected ── */ | ||
| .connected { margin-top: 22px; display: flex; flex-direction: column; gap: 14px; } | ||
| .conn-status { display: inline-flex; align-items: center; gap: 8px; font-size: 13.5px; font-weight: 600; color: var(--color-success); } | ||
| .conn-dot { width: 8px; height: 8px; border-radius: var(--radius-pill); background: var(--color-success); } | ||
| .conn-reminder { | ||
| font-size: 12px; line-height: 1.5; color: var(--color-foreground); | ||
| background: color-mix(in srgb, var(--color-primary) 10%, transparent); | ||
| border: 1px solid color-mix(in srgb, var(--color-primary) 30%, transparent); | ||
| border-radius: var(--radius-lg); padding: 10px 12px; | ||
| } | ||
| .btn-outline { | ||
| align-self: flex-start; | ||
| padding: 7px 14px; | ||
| font-size: 12.5px; | ||
| font-weight: 500; | ||
| border-radius: var(--radius-lg); | ||
| border: 1px solid var(--color-border); | ||
| background: var(--color-surface); | ||
| color: var(--color-foreground); | ||
| cursor: pointer; | ||
| transition: background 0.15s; | ||
| } | ||
| .btn-outline:hover:not(:disabled) { background: var(--color-muted); } | ||
| .btn-outline:disabled { opacity: 0.6; cursor: not-allowed; } | ||
|
|
||
| /* ── Phone mock (right) ── */ | ||
| .phone-col { flex: 0 0 auto; display: flex; padding-top: 24px; } | ||
| .phone { | ||
| width: 240px; | ||
| border: 2px solid var(--color-foreground); | ||
| border-radius: 32px; | ||
| padding: 10px; | ||
| background: var(--color-background); | ||
| box-shadow: var(--shadow-xl); | ||
| position: relative; | ||
| } | ||
| .phone-notch { | ||
| position: absolute; top: 10px; left: 50%; transform: translateX(-50%); | ||
| width: 72px; height: 18px; | ||
| background: var(--color-foreground); | ||
| border-radius: 0 0 12px 12px; | ||
| } | ||
| .phone-screen { | ||
| border-radius: 24px; | ||
| overflow: hidden; | ||
| background: var(--color-muted); | ||
| height: 400px; | ||
| padding: 32px 12px 14px; | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 11px; | ||
| } | ||
| .phone-status { display: flex; align-items: center; justify-content: space-between; font-size: 9px; color: var(--color-muted-foreground); padding: 0 6px; } | ||
| .bat { width: 16px; height: 8px; border: 1px solid currentColor; border-radius: 2px; position: relative; } | ||
| .bat::after { content: ''; position: absolute; inset: 1px; background: currentColor; border-radius: 1px; } | ||
|
|
||
| .wc-msg, .wc-msg-2 { background: var(--color-surface); border: 1px solid var(--color-border); border-radius: 12px; } | ||
| .wc-msg { padding: 12px 13px; box-shadow: var(--shadow-sm); } | ||
| .wc-head { display: flex; align-items: center; gap: 7px; margin-bottom: 8px; } | ||
| .wc-avatar { width: 22px; height: 22px; border-radius: 6px; background: var(--color-primary); display: grid; place-items: center; color: #fff; font-size: 10px; font-weight: 700; font-family: var(--font-mono); } | ||
| .wc-name { font-size: 11.5px; font-weight: 600; } | ||
| .wc-time { margin-left: auto; font-size: 9px; color: var(--color-muted-foreground); } | ||
| .wc-body { font-size: 11.5px; line-height: 1.5; color: var(--color-foreground); } | ||
| .wc-body .muted { color: var(--color-muted-foreground); } | ||
| .wc-body code { font-family: var(--font-mono); font-size: 10px; background: var(--color-muted); padding: 1px 4px; border-radius: 3px; } | ||
| .wc-actions { display: flex; gap: 8px; margin-top: 10px; } | ||
| .wc-btn { flex: 1; padding: 8px 0; border-radius: 8px; font-size: 11px; font-weight: 600; border: none; cursor: default; } | ||
| .wc-btn.approve { background: #07C160; color: #fff; } | ||
| .wc-btn.deny { background: var(--color-muted); color: var(--color-foreground); border: 1px solid var(--color-border); } | ||
| .wc-msg-2 { padding: 10px 13px; } | ||
| .wc-sub { font-size: 10px; color: var(--color-muted-foreground); margin-bottom: 3px; } | ||
| .wc-line { font-size: 11.5px; font-weight: 500; } | ||
| .wc-pill { display: inline-flex; align-items: center; gap: 4px; font-size: 10px; font-weight: 600; color: #07C160; background: color-mix(in srgb, #07C160 12%, transparent); padding: 2px 8px; border-radius: var(--radius-pill); margin-top: 7px; } | ||
| </style> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace hardcoded colors with design tokens.
The phone mock CSS contains hardcoded hex colors that violate the coding guideline requiring all colors to come from tokens.css:
- Line 309:
#ffffallback invar(--color-on-primary,#fff) - Line 394:
#07C160(WeChat green) and#fff - Line 399:
#07C160in pill background
Replace these with appropriate design tokens.
🎨 Recommended fixes
.btn-primary {
display: inline-flex; align-items: center; gap: 8px;
padding: 10px 18px;
background: var(--color-primary);
- color: var(--color-on-primary, `#fff`);
+ color: var(--color-on-primary);
border: none;
border-radius: var(--radius-pill);For the WeChat mock buttons, replace the hardcoded brand green with a success token:
.wc-btn { flex: 1; padding: 8px 0; border-radius: 8px; font-size: 11px; font-weight: 600; border: none; cursor: default; }
-.wc-btn.approve { background: `#07C160`; color: `#fff`; }
+.wc-btn.approve { background: var(--color-success); color: var(--color-on-primary); }
.wc-btn.deny { background: var(--color-muted); color: var(--color-foreground); border: 1px solid var(--color-border); }-.wc-pill { display: inline-flex; align-items: center; gap: 4px; font-size: 10px; font-weight: 600; color: `#07C160`; background: color-mix(in srgb, `#07C160` 12%, transparent); padding: 2px 8px; border-radius: var(--radius-pill); margin-top: 7px; }
+.wc-pill { display: inline-flex; align-items: center; gap: 4px; font-size: 10px; font-weight: 600; color: var(--color-success); background: color-mix(in srgb, var(--color-success) 12%, transparent); padding: 2px 8px; border-radius: var(--radius-pill); margin-top: 7px; }As per coding guidelines web/**/*.{vue,css}: Every color must come from a CSS custom property defined in src/styles/tokens.css. Never hardcode hex/rgb/#fff/white in .vue or .css.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <style scoped> | |
| /* The page surface (inset chrome + title head + scroll body) is owned by | |
| * PageSurface. This component styles only its own content. Content column is | |
| * centered + inset to match the chat timeline, scoped to PageSurface's body. */ | |
| :deep(.page-body) > * { max-width: 56rem; margin-left: auto; margin-right: auto; padding: 0 20px 32px; } | |
| .chan-stage { display: flex; gap: 40px; align-items: flex-start; padding-top: 16px; } | |
| @media (max-width: 860px) { .chan-stage { flex-direction: column; } } | |
| /* ── Promo card (left-anchored) ── */ | |
| .promo-card { | |
| flex: 1; | |
| min-width: 0; | |
| position: relative; | |
| background: | |
| radial-gradient(120% 80% at 0% 0%, color-mix(in srgb, var(--color-primary) 8%, transparent), transparent 60%), | |
| var(--color-background); | |
| border: 1px solid var(--color-border); | |
| border-radius: var(--radius-2xl); | |
| overflow: hidden; | |
| } | |
| .promo-glow { | |
| position: absolute; | |
| top: -90px; left: -60px; | |
| width: 280px; height: 280px; | |
| border-radius: 50%; | |
| background: color-mix(in srgb, var(--color-primary) 13%, transparent); | |
| filter: blur(46px); | |
| opacity: 0.8; | |
| pointer-events: none; | |
| } | |
| .promo-inner { position: relative; padding: 32px; display: flex; flex-direction: column; } | |
| .promo-eyebrow { | |
| align-self: flex-start; | |
| display: inline-flex; | |
| align-items: center; | |
| gap: 7px; | |
| font-size: 11px; | |
| font-weight: 600; | |
| letter-spacing: 0.04em; | |
| text-transform: uppercase; | |
| color: var(--color-primary); | |
| padding: 5px 11px; | |
| border-radius: var(--radius-pill); | |
| background: color-mix(in srgb, var(--color-primary) 8%, transparent); | |
| border: 1px solid color-mix(in srgb, var(--color-primary) 35%, transparent); | |
| } | |
| .promo-title { | |
| font-size: 26px; | |
| font-weight: 600; | |
| letter-spacing: -0.02em; | |
| margin: 18px 0 0; | |
| line-height: 1.15; | |
| } | |
| .promo-title :deep(.accent) { color: var(--color-primary); } | |
| .promo-lede { | |
| font-size: 13.5px; | |
| color: var(--color-muted-foreground); | |
| line-height: 1.6; | |
| margin: 14px 0 0; | |
| max-width: 44ch; | |
| } | |
| .promo-features { display: flex; flex-direction: column; gap: 16px; margin: 24px 0 0; } | |
| .promo-feat { display: flex; gap: 13px; align-items: flex-start; } | |
| .feat-ic { | |
| display: grid; place-items: center; | |
| width: 36px; height: 36px; flex-shrink: 0; | |
| border-radius: var(--radius-lg); | |
| background: color-mix(in srgb, var(--color-foreground) 8%, transparent); | |
| border: 1px solid color-mix(in srgb, var(--color-foreground) 30%, transparent); | |
| color: var(--color-foreground); | |
| } | |
| .promo-feat h4 { font-size: 13.5px; font-weight: 600; margin: 3px 0 3px; } | |
| .promo-feat p { font-size: 12.5px; color: var(--color-muted-foreground); line-height: 1.5; margin: 0; max-width: 42ch; } | |
| .promo-cta { display: flex; align-items: center; gap: 14px; margin-top: 26px; flex-wrap: wrap; } | |
| .btn-primary { | |
| display: inline-flex; align-items: center; gap: 8px; | |
| padding: 10px 18px; | |
| background: var(--color-primary); | |
| color: var(--color-on-primary, #fff); | |
| border: none; | |
| border-radius: var(--radius-pill); | |
| font-size: 13px; | |
| font-weight: 600; | |
| cursor: pointer; | |
| box-shadow: 0 4px 16px -4px color-mix(in srgb, var(--color-primary) 60%, transparent); | |
| transition: box-shadow 0.15s, opacity 0.15s; | |
| } | |
| .btn-primary:hover:not(:disabled) { box-shadow: 0 6px 22px -5px color-mix(in srgb, var(--color-primary) 70%, transparent); } | |
| .btn-primary:disabled { opacity: 0.6; cursor: not-allowed; } | |
| .cta-hint { font-size: 11.5px; color: var(--color-muted-foreground); } | |
| /* ── Connect (QR) ── */ | |
| .qr-block { display: flex; flex-direction: column; align-items: center; gap: 10px; margin-top: 22px; } | |
| .qr-canvas { border: 1px solid var(--color-border); border-radius: var(--radius-md); } | |
| .qr-hint { font-size: 12px; color: var(--color-muted-foreground); } | |
| /* ── Connected ── */ | |
| .connected { margin-top: 22px; display: flex; flex-direction: column; gap: 14px; } | |
| .conn-status { display: inline-flex; align-items: center; gap: 8px; font-size: 13.5px; font-weight: 600; color: var(--color-success); } | |
| .conn-dot { width: 8px; height: 8px; border-radius: var(--radius-pill); background: var(--color-success); } | |
| .conn-reminder { | |
| font-size: 12px; line-height: 1.5; color: var(--color-foreground); | |
| background: color-mix(in srgb, var(--color-primary) 10%, transparent); | |
| border: 1px solid color-mix(in srgb, var(--color-primary) 30%, transparent); | |
| border-radius: var(--radius-lg); padding: 10px 12px; | |
| } | |
| .btn-outline { | |
| align-self: flex-start; | |
| padding: 7px 14px; | |
| font-size: 12.5px; | |
| font-weight: 500; | |
| border-radius: var(--radius-lg); | |
| border: 1px solid var(--color-border); | |
| background: var(--color-surface); | |
| color: var(--color-foreground); | |
| cursor: pointer; | |
| transition: background 0.15s; | |
| } | |
| .btn-outline:hover:not(:disabled) { background: var(--color-muted); } | |
| .btn-outline:disabled { opacity: 0.6; cursor: not-allowed; } | |
| /* ── Phone mock (right) ── */ | |
| .phone-col { flex: 0 0 auto; display: flex; padding-top: 24px; } | |
| .phone { | |
| width: 240px; | |
| border: 2px solid var(--color-foreground); | |
| border-radius: 32px; | |
| padding: 10px; | |
| background: var(--color-background); | |
| box-shadow: var(--shadow-xl); | |
| position: relative; | |
| } | |
| .phone-notch { | |
| position: absolute; top: 10px; left: 50%; transform: translateX(-50%); | |
| width: 72px; height: 18px; | |
| background: var(--color-foreground); | |
| border-radius: 0 0 12px 12px; | |
| } | |
| .phone-screen { | |
| border-radius: 24px; | |
| overflow: hidden; | |
| background: var(--color-muted); | |
| height: 400px; | |
| padding: 32px 12px 14px; | |
| display: flex; | |
| flex-direction: column; | |
| gap: 11px; | |
| } | |
| .phone-status { display: flex; align-items: center; justify-content: space-between; font-size: 9px; color: var(--color-muted-foreground); padding: 0 6px; } | |
| .bat { width: 16px; height: 8px; border: 1px solid currentColor; border-radius: 2px; position: relative; } | |
| .bat::after { content: ''; position: absolute; inset: 1px; background: currentColor; border-radius: 1px; } | |
| .wc-msg, .wc-msg-2 { background: var(--color-surface); border: 1px solid var(--color-border); border-radius: 12px; } | |
| .wc-msg { padding: 12px 13px; box-shadow: var(--shadow-sm); } | |
| .wc-head { display: flex; align-items: center; gap: 7px; margin-bottom: 8px; } | |
| .wc-avatar { width: 22px; height: 22px; border-radius: 6px; background: var(--color-primary); display: grid; place-items: center; color: #fff; font-size: 10px; font-weight: 700; font-family: var(--font-mono); } | |
| .wc-name { font-size: 11.5px; font-weight: 600; } | |
| .wc-time { margin-left: auto; font-size: 9px; color: var(--color-muted-foreground); } | |
| .wc-body { font-size: 11.5px; line-height: 1.5; color: var(--color-foreground); } | |
| .wc-body .muted { color: var(--color-muted-foreground); } | |
| .wc-body code { font-family: var(--font-mono); font-size: 10px; background: var(--color-muted); padding: 1px 4px; border-radius: 3px; } | |
| .wc-actions { display: flex; gap: 8px; margin-top: 10px; } | |
| .wc-btn { flex: 1; padding: 8px 0; border-radius: 8px; font-size: 11px; font-weight: 600; border: none; cursor: default; } | |
| .wc-btn.approve { background: #07C160; color: #fff; } | |
| .wc-btn.deny { background: var(--color-muted); color: var(--color-foreground); border: 1px solid var(--color-border); } | |
| .wc-msg-2 { padding: 10px 13px; } | |
| .wc-sub { font-size: 10px; color: var(--color-muted-foreground); margin-bottom: 3px; } | |
| .wc-line { font-size: 11.5px; font-weight: 500; } | |
| .wc-pill { display: inline-flex; align-items: center; gap: 4px; font-size: 10px; font-weight: 600; color: #07C160; background: color-mix(in srgb, #07C160 12%, transparent); padding: 2px 8px; border-radius: var(--radius-pill); margin-top: 7px; } | |
| </style> | |
| <style scoped> | |
| /* The page surface (inset chrome + title head + scroll body) is owned by | |
| * PageSurface. This component styles only its own content. Content column is | |
| * centered + inset to match the chat timeline, scoped to PageSurface's body. */ | |
| :deep(.page-body) > * { max-width: 56rem; margin-left: auto; margin-right: auto; padding: 0 20px 32px; } | |
| .chan-stage { display: flex; gap: 40px; align-items: flex-start; padding-top: 16px; } | |
| `@media` (max-width: 860px) { .chan-stage { flex-direction: column; } } | |
| /* ── Promo card (left-anchored) ── */ | |
| .promo-card { | |
| flex: 1; | |
| min-width: 0; | |
| position: relative; | |
| background: | |
| radial-gradient(120% 80% at 0% 0%, color-mix(in srgb, var(--color-primary) 8%, transparent), transparent 60%), | |
| var(--color-background); | |
| border: 1px solid var(--color-border); | |
| border-radius: var(--radius-2xl); | |
| overflow: hidden; | |
| } | |
| .promo-glow { | |
| position: absolute; | |
| top: -90px; left: -60px; | |
| width: 280px; height: 280px; | |
| border-radius: 50%; | |
| background: color-mix(in srgb, var(--color-primary) 13%, transparent); | |
| filter: blur(46px); | |
| opacity: 0.8; | |
| pointer-events: none; | |
| } | |
| .promo-inner { position: relative; padding: 32px; display: flex; flex-direction: column; } | |
| .promo-eyebrow { | |
| align-self: flex-start; | |
| display: inline-flex; | |
| align-items: center; | |
| gap: 7px; | |
| font-size: 11px; | |
| font-weight: 600; | |
| letter-spacing: 0.04em; | |
| text-transform: uppercase; | |
| color: var(--color-primary); | |
| padding: 5px 11px; | |
| border-radius: var(--radius-pill); | |
| background: color-mix(in srgb, var(--color-primary) 8%, transparent); | |
| border: 1px solid color-mix(in srgb, var(--color-primary) 35%, transparent); | |
| } | |
| .promo-title { | |
| font-size: 26px; | |
| font-weight: 600; | |
| letter-spacing: -0.02em; | |
| margin: 18px 0 0; | |
| line-height: 1.15; | |
| } | |
| .promo-title :deep(.accent) { color: var(--color-primary); } | |
| .promo-lede { | |
| font-size: 13.5px; | |
| color: var(--color-muted-foreground); | |
| line-height: 1.6; | |
| margin: 14px 0 0; | |
| max-width: 44ch; | |
| } | |
| .promo-features { display: flex; flex-direction: column; gap: 16px; margin: 24px 0 0; } | |
| .promo-feat { display: flex; gap: 13px; align-items: flex-start; } | |
| .feat-ic { | |
| display: grid; place-items: center; | |
| width: 36px; height: 36px; flex-shrink: 0; | |
| border-radius: var(--radius-lg); | |
| background: color-mix(in srgb, var(--color-foreground) 8%, transparent); | |
| border: 1px solid color-mix(in srgb, var(--color-foreground) 30%, transparent); | |
| color: var(--color-foreground); | |
| } | |
| .promo-feat h4 { font-size: 13.5px; font-weight: 600; margin: 3px 0 3px; } | |
| .promo-feat p { font-size: 12.5px; color: var(--color-muted-foreground); line-height: 1.5; margin: 0; max-width: 42ch; } | |
| .promo-cta { display: flex; align-items: center; gap: 14px; margin-top: 26px; flex-wrap: wrap; } | |
| .btn-primary { | |
| display: inline-flex; align-items: center; gap: 8px; | |
| padding: 10px 18px; | |
| background: var(--color-primary); | |
| color: var(--color-on-primary); | |
| border: none; | |
| border-radius: var(--radius-pill); | |
| font-size: 13px; | |
| font-weight: 600; | |
| cursor: pointer; | |
| box-shadow: 0 4px 16px -4px color-mix(in srgb, var(--color-primary) 60%, transparent); | |
| transition: box-shadow 0.15s, opacity 0.15s; | |
| } | |
| .btn-primary:hover:not(:disabled) { box-shadow: 0 6px 22px -5px color-mix(in srgb, var(--color-primary) 70%, transparent); } | |
| .btn-primary:disabled { opacity: 0.6; cursor: not-allowed; } | |
| .cta-hint { font-size: 11.5px; color: var(--color-muted-foreground); } | |
| /* ── Connect (QR) ── */ | |
| .qr-block { display: flex; flex-direction: column; align-items: center; gap: 10px; margin-top: 22px; } | |
| .qr-canvas { border: 1px solid var(--color-border); border-radius: var(--radius-md); } | |
| .qr-hint { font-size: 12px; color: var(--color-muted-foreground); } | |
| /* ── Connected ── */ | |
| .connected { margin-top: 22px; display: flex; flex-direction: column; gap: 14px; } | |
| .conn-status { display: inline-flex; align-items: center; gap: 8px; font-size: 13.5px; font-weight: 600; color: var(--color-success); } | |
| .conn-dot { width: 8px; height: 8px; border-radius: var(--radius-pill); background: var(--color-success); } | |
| .conn-reminder { | |
| font-size: 12px; line-height: 1.5; color: var(--color-foreground); | |
| background: color-mix(in srgb, var(--color-primary) 10%, transparent); | |
| border: 1px solid color-mix(in srgb, var(--color-primary) 30%, transparent); | |
| border-radius: var(--radius-lg); padding: 10px 12px; | |
| } | |
| .btn-outline { | |
| align-self: flex-start; | |
| padding: 7px 14px; | |
| font-size: 12.5px; | |
| font-weight: 500; | |
| border-radius: var(--radius-lg); | |
| border: 1px solid var(--color-border); | |
| background: var(--color-surface); | |
| color: var(--color-foreground); | |
| cursor: pointer; | |
| transition: background 0.15s; | |
| } | |
| .btn-outline:hover:not(:disabled) { background: var(--color-muted); } | |
| .btn-outline:disabled { opacity: 0.6; cursor: not-allowed; } | |
| /* ── Phone mock (right) ── */ | |
| .phone-col { flex: 0 0 auto; display: flex; padding-top: 24px; } | |
| .phone { | |
| width: 240px; | |
| border: 2px solid var(--color-foreground); | |
| border-radius: 32px; | |
| padding: 10px; | |
| background: var(--color-background); | |
| box-shadow: var(--shadow-xl); | |
| position: relative; | |
| } | |
| .phone-notch { | |
| position: absolute; top: 10px; left: 50%; transform: translateX(-50%); | |
| width: 72px; height: 18px; | |
| background: var(--color-foreground); | |
| border-radius: 0 0 12px 12px; | |
| } | |
| .phone-screen { | |
| border-radius: 24px; | |
| overflow: hidden; | |
| background: var(--color-muted); | |
| height: 400px; | |
| padding: 32px 12px 14px; | |
| display: flex; | |
| flex-direction: column; | |
| gap: 11px; | |
| } | |
| .phone-status { display: flex; align-items: center; justify-content: space-between; font-size: 9px; color: var(--color-muted-foreground); padding: 0 6px; } | |
| .bat { width: 16px; height: 8px; border: 1px solid currentColor; border-radius: 2px; position: relative; } | |
| .bat::after { content: ''; position: absolute; inset: 1px; background: currentColor; border-radius: 1px; } | |
| .wc-msg, .wc-msg-2 { background: var(--color-surface); border: 1px solid var(--color-border); border-radius: 12px; } | |
| .wc-msg { padding: 12px 13px; box-shadow: var(--shadow-sm); } | |
| .wc-head { display: flex; align-items: center; gap: 7px; margin-bottom: 8px; } | |
| .wc-avatar { width: 22px; height: 22px; border-radius: 6px; background: var(--color-primary); display: grid; place-items: center; color: `#fff`; font-size: 10px; font-weight: 700; font-family: var(--font-mono); } | |
| .wc-name { font-size: 11.5px; font-weight: 600; } | |
| .wc-time { margin-left: auto; font-size: 9px; color: var(--color-muted-foreground); } | |
| .wc-body { font-size: 11.5px; line-height: 1.5; color: var(--color-foreground); } | |
| .wc-body .muted { color: var(--color-muted-foreground); } | |
| .wc-body code { font-family: var(--font-mono); font-size: 10px; background: var(--color-muted); padding: 1px 4px; border-radius: 3px; } | |
| .wc-actions { display: flex; gap: 8px; margin-top: 10px; } | |
| .wc-btn { flex: 1; padding: 8px 0; border-radius: 8px; font-size: 11px; font-weight: 600; border: none; cursor: default; } | |
| .wc-btn.approve { background: var(--color-success); color: var(--color-on-primary); } | |
| .wc-btn.deny { background: var(--color-muted); color: var(--color-foreground); border: 1px solid var(--color-border); } | |
| .wc-msg-2 { padding: 10px 13px; } | |
| .wc-sub { font-size: 10px; color: var(--color-muted-foreground); margin-bottom: 3px; } | |
| .wc-line { font-size: 11.5px; font-weight: 500; } | |
| .wc-pill { display: inline-flex; align-items: center; gap: 4px; font-size: 10px; font-weight: 600; color: var(--color-success); background: color-mix(in srgb, var(--color-success) 12%, transparent); padding: 2px 8px; border-radius: var(--radius-pill); margin-top: 7px; } | |
| </style> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/ChannelsView.vue` around lines 228 - 400, The CSS in the
style block contains multiple hardcoded hex color values that violate the coding
guideline requiring all colors to come from design tokens. Replace the hardcoded
colors in the following locations: the fallback `#fff` in the .btn-primary class
color property, the `#07C160` and `#fff` hardcoded values in the .wc-btn.approve
class, and the `#07C160` used in the .wc-pill class color and background color-mix
function. Use appropriate design tokens from tokens.css (such as --color-success
for the WeChat green, --color-on-primary for the white/light color, and similar
tokens) to replace all hardcoded hex values.
Source: Coding guidelines
| <!-- Nav header — a whitespace index (the "C2" design: no lines, just air). | ||
| Three equal controls float on the sidebar background; a row only gains | ||
| a body on hover/active, so at rest there's no cage of borders. Each | ||
| carries its shortcut so it's discoverable without a tooltip. "New task" | ||
| returns to chat (and starts a session); the other two are page views. --> | ||
| <div class="sidebar-header"> | ||
| <button class="new-task-btn" @click="store.newSession()"> | ||
| <PlusIcon class="w-4 h-4" /> | ||
| <span>{{ t('nav.newTask') }}</span> | ||
| </button> | ||
| <div class="nav-list"> | ||
| <button class="nav-row" @click="newTask"> | ||
| <PlusIcon class="nav-ic" /> | ||
| <span class="nav-name">{{ t('nav.newTask') }}</span> | ||
| <span class="nav-kbd">{{ platformMod }}N</span> | ||
| </button> | ||
| <button | ||
| class="nav-row" | ||
| :class="{ active: activeView === 'automations' }" | ||
| @click="emit('openAutomations')" | ||
| :aria-current="activeView === 'automations' ? 'page' : undefined" | ||
| > | ||
| <SparklesIcon class="nav-ic" /> | ||
| <span class="nav-name">{{ t('nav.automations') }}</span> | ||
| <span class="nav-kbd">{{ platformMod }}A</span> | ||
| </button> | ||
| <button | ||
| class="nav-row" | ||
| :class="{ active: activeView === 'channels' }" | ||
| @click="emit('openChannels')" | ||
| :aria-current="activeView === 'channels' ? 'page' : undefined" | ||
| > | ||
| <SignalIcon class="nav-ic" /> | ||
| <span class="nav-name">{{ t('nav.channels') }}</span> | ||
| <span class="nav-kbd">{{ platformMod }}C</span> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use Tailwind sizing classes for nav header icons.
The nav header icons (lines 434, 444, 454) use the .nav-ic CSS class for sizing (width: 18px; height: 18px;) instead of Tailwind utilities, which is inconsistent with the rest of the file (e.g., lines 483, 484, 500) and violates the coding guideline.
♻️ Recommended fix to use Tailwind sizing
Update the icon elements to include Tailwind sizing classes:
- <PlusIcon class="nav-ic" />
+ <PlusIcon class="w-[18px] h-[18px] nav-ic" />
<span class="nav-name">{{ t('nav.newTask') }}</span>
<span class="nav-kbd">{{ platformMod }}N</span>
</button>
<button
class="nav-row"
:class="{ active: activeView === 'automations' }"
`@click`="emit('openAutomations')"
:aria-current="activeView === 'automations' ? 'page' : undefined"
>
- <SparklesIcon class="nav-ic" />
+ <SparklesIcon class="w-[18px] h-[18px] nav-ic" />
<span class="nav-name">{{ t('nav.automations') }}</span>
<span class="nav-kbd">{{ platformMod }}A</span>
</button>
<button
class="nav-row"
:class="{ active: activeView === 'channels' }"
`@click`="emit('openChannels')"
:aria-current="activeView === 'channels' ? 'page' : undefined"
>
- <SignalIcon class="nav-ic" />
+ <SignalIcon class="w-[18px] h-[18px] nav-ic" />Then remove the width/height from .nav-ic in the CSS (lines 674-679):
.nav-ic {
- width: 18px;
- height: 18px;
flex-shrink: 0;
color: var(--color-muted-foreground);
transition: color 0.15s;
}As per coding guidelines web/**/*.vue: Use Tailwind w-N h-N classes for icon sizing, never a :size prop.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <!-- Nav header — a whitespace index (the "C2" design: no lines, just air). | |
| Three equal controls float on the sidebar background; a row only gains | |
| a body on hover/active, so at rest there's no cage of borders. Each | |
| carries its shortcut so it's discoverable without a tooltip. "New task" | |
| returns to chat (and starts a session); the other two are page views. --> | |
| <div class="sidebar-header"> | |
| <button class="new-task-btn" @click="store.newSession()"> | |
| <PlusIcon class="w-4 h-4" /> | |
| <span>{{ t('nav.newTask') }}</span> | |
| </button> | |
| <div class="nav-list"> | |
| <button class="nav-row" @click="newTask"> | |
| <PlusIcon class="nav-ic" /> | |
| <span class="nav-name">{{ t('nav.newTask') }}</span> | |
| <span class="nav-kbd">{{ platformMod }}N</span> | |
| </button> | |
| <button | |
| class="nav-row" | |
| :class="{ active: activeView === 'automations' }" | |
| @click="emit('openAutomations')" | |
| :aria-current="activeView === 'automations' ? 'page' : undefined" | |
| > | |
| <SparklesIcon class="nav-ic" /> | |
| <span class="nav-name">{{ t('nav.automations') }}</span> | |
| <span class="nav-kbd">{{ platformMod }}A</span> | |
| </button> | |
| <button | |
| class="nav-row" | |
| :class="{ active: activeView === 'channels' }" | |
| @click="emit('openChannels')" | |
| :aria-current="activeView === 'channels' ? 'page' : undefined" | |
| > | |
| <SignalIcon class="nav-ic" /> | |
| <span class="nav-name">{{ t('nav.channels') }}</span> | |
| <span class="nav-kbd">{{ platformMod }}C</span> | |
| </button> | |
| </div> | |
| <!-- Nav header — a whitespace index (the "C2" design: no lines, just air). | |
| Three equal controls float on the sidebar background; a row only gains | |
| a body on hover/active, so at rest there's no cage of borders. Each | |
| carries its shortcut so it's discoverable without a tooltip. "New task" | |
| returns to chat (and starts a session); the other two are page views. --> | |
| <div class="sidebar-header"> | |
| <div class="nav-list"> | |
| <button class="nav-row" `@click`="newTask"> | |
| <PlusIcon class="w-[18px] h-[18px] nav-ic" /> | |
| <span class="nav-name">{{ t('nav.newTask') }}</span> | |
| <span class="nav-kbd">{{ platformMod }}N</span> | |
| </button> | |
| <button | |
| class="nav-row" | |
| :class="{ active: activeView === 'automations' }" | |
| `@click`="emit('openAutomations')" | |
| :aria-current="activeView === 'automations' ? 'page' : undefined" | |
| > | |
| <SparklesIcon class="w-[18px] h-[18px] nav-ic" /> | |
| <span class="nav-name">{{ t('nav.automations') }}</span> | |
| <span class="nav-kbd">{{ platformMod }}A</span> | |
| </button> | |
| <button | |
| class="nav-row" | |
| :class="{ active: activeView === 'channels' }" | |
| `@click`="emit('openChannels')" | |
| :aria-current="activeView === 'channels' ? 'page' : undefined" | |
| > | |
| <SignalIcon class="w-[18px] h-[18px] nav-ic" /> | |
| <span class="nav-name">{{ t('nav.channels') }}</span> | |
| <span class="nav-kbd">{{ platformMod }}C</span> | |
| </button> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/Sidebar.vue` around lines 426 - 458, The nav-ic class is
being used to size the PlusIcon, SparklesIcon, and SignalIcon elements in the
sidebar header nav-row buttons, which violates the coding guideline to use
Tailwind utilities for icon sizing. Replace the nav-ic class usage on each of
these icon elements with appropriate Tailwind sizing classes (such as w-5 h-5 or
w-4 h-4 depending on the intended size), then remove the width and height CSS
rules from the nav-ic class definition in the stylesheet to complete the
migration to Tailwind utilities.
Source: Coding guidelines
| nav: { | ||
| newTask: '新增工作', | ||
| automations: '自動化', | ||
| channels: '通道', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep zh-Hant channel terminology consistent across navigation and settings paths.
Line 38 / Line 66 use 通道, while the same destination is labeled 頻道 in settings.tabs.channels; this can confuse users in path-like copy (“設定 → …”).
Also applies to: 66-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/i18n/locales/zh-Hant.ts` at line 38, The Traditional Chinese
translations for "channels" terminology are inconsistent across the file. The
entries at lines 38 and 66 use `通道`, but the `settings.tabs.channels` key uses
`頻道` for the same concept, which creates confusion in navigation and settings
paths for users. Identify all occurrences where "channels" is translated in the
zh-Hant.ts file (including the `channels` key around line 38, the second
occurrence around line 66, and the `settings.tabs.channels` entry) and ensure
they all use the same consistent Chinese term throughout the file.
…iew component - Created a new test file `web_tools_test.go` to verify the functionality of dropping interactive tools from the automation tool set. - Implemented the `dropInteractiveTools` function to filter out tools that require user interaction. - Added a new Vue component `AutomationRunView.vue` to display the details of an automation run, including a timeline of messages and tool calls. - The component handles loading states, error messages, and provides actions to run the automation again or stop the current run.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/command/web.go (1)
355-373: 🩺 Stability & Availability | 🟡 Minor
buildPlanToolsshould dropask_usertoo.runAutomationkeeps the configured mode for non-scheduled runs, andplanis a valid automation mode, so a headless automation can still rebuild withplanMode=trueand reintroduce an interactive tool. Apply the same filtering there, or gateNewAskUserToolon the unattended path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/command/web.go` around lines 355 - 373, buildPlanTools currently always includes the interactive ask_user tool, which can leak back into unattended plan-mode automation. Update the buildPlanTools helper in web.go to apply the same interactive-tool filtering used by dropInteractiveTools, or conditionally skip tools.NewAskUserTool when running in the unattended path so planMode cannot reintroduce an interactive tool.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@design/automations-redesign.html`:
- Line 1000: The switch markup uses self-closing spans, which is invalid for
non-void elements and can lead to parser-dependent DOM structure. Update the
repeated switch templates in the automation redesign HTML so the switch-knob
element uses an explicit opening and closing span tag, and apply the same fix to
every matching instance in the affected markup.
- Line 1036: The automation title contains a typo in the card-name text used for
the stale issues workflow. Update the string in the relevant card markup so
“Triange stale issues” becomes “Triage stale issues”, and make the same
correction in the other duplicate occurrence referenced by the review comment.
- Line 905: The `.btn-danger` styles are using `var(--color-on-destructive)`
before that custom property is defined, so update the theme/token definitions to
declare `--color-on-destructive` first and ensure it is available where
`.btn-danger` is applied. Check the CSS variable block that defines the color
palette in the same stylesheet and add the missing token there, keeping the
existing `background: var(--color-destructive)` usage unchanged.
In `@web/src/App.vue`:
- Around line 50-53: The active run detail is holding a stale run snapshot
instead of tracking the refreshed store state. Update the App.vue flow around
activeRun, showRunDetails, and the run detail binding so the selected run is
derived from automationStore.runs (matched by id) whenever the store refreshes,
rather than keeping the originally clicked object. Make sure the detail header
and status/duration/error fields always read from the current store record, and
adjust any related computed/watch logic used in the active run selection.
In `@web/src/components/AutomationRunView.vue`:
- Line 315: The inline color fallback in AutomationRunView’s status-chip styles
still hardcodes a color-mix value instead of relying on design tokens. Remove
the component-level fallback from the .status-chip.running and related
status-chip rules, switch them to token-only custom properties, and add the
missing accent token definitions in src/styles/tokens.css so all colors are
sourced from tokens only.
- Around line 120-121: The replay cleanup in applyEntries is incorrectly
finalizing unmatched tool_call entries, which breaks live sessions by marking
in-flight calls as done too early. Update AutomationRunView’s applyEntries
handling so only finished-run replays close out unresolved pending tool calls,
and leave them active during live sessions until their result entry arrives. Use
the pending collection logic in applyEntries and the tool_call status update
path to scope this behavior correctly.
- Around line 151-158: The stopRun() handler in AutomationRunView.vue is forcing
running.value to false even when api.stop(props.run.session_id) fails, which
makes a failed stop look like the run stopped successfully. Update stopRun() so
the catch path surfaces the error instead of changing the UI to stopped, and let
the existing polling/state refresh confirm when the run has actually terminated.
Keep the success path in stopRun() unchanged and preserve the run state until
the API call succeeds or polling updates it.
---
Outside diff comments:
In `@internal/command/web.go`:
- Around line 355-373: buildPlanTools currently always includes the interactive
ask_user tool, which can leak back into unattended plan-mode automation. Update
the buildPlanTools helper in web.go to apply the same interactive-tool filtering
used by dropInteractiveTools, or conditionally skip tools.NewAskUserTool when
running in the unattended path so planMode cannot reintroduce an interactive
tool.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adab1339-3da2-46af-b529-88ce5d8cd88f
📒 Files selected for processing (12)
design/automations-redesign.htmlinternal/command/web.gointernal/command/web_tools_test.gointernal/web/automation_run.gointernal/web/engine.gointernal/web/server.goweb/src/App.vueweb/src/components/AutomationRunView.vueweb/src/components/AutomationsView.vueweb/src/components/ChannelsView.vueweb/src/components/PageSurface.vueweb/src/components/ProjectPickerPanel.vue
💤 Files with no reviewable changes (1)
- web/src/components/ProjectPickerPanel.vue
🚧 Files skipped from review as they are similar to previous changes (5)
- web/src/components/PageSurface.vue
- internal/web/automation_run.go
- web/src/components/ChannelsView.vue
- web/src/components/AutomationsView.vue
- internal/web/server.go
| display: inline-flex; align-items: center; gap: 6px; | ||
| padding: 6px 12px; font-family: inherit; font-size: 12.5px; font-weight: 500; | ||
| border: none; border-radius: var(--radius-lg); | ||
| background: var(--color-destructive); color: var(--color-on-destructive); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define --color-on-destructive before using it in .btn-danger.
color: var(--color-on-destructive) is currently unresolved, so the text color can fall back unexpectedly.
Suggested fix
:root {
+ --color-on-destructive: `#FFFFFF`;
...
}
[data-theme='dark'] {
+ --color-on-destructive: `#FFFFFF`;
...
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design/automations-redesign.html` at line 905, The `.btn-danger` styles are
using `var(--color-on-destructive)` before that custom property is defined, so
update the theme/token definitions to declare `--color-on-destructive` first and
ensure it is available where `.btn-danger` is applied. Check the CSS variable
block that defines the color palette in the same stylesheet and add the missing
token there, keeping the existing `background: var(--color-destructive)` usage
unchanged.
| <span class="card-actions" style="grid-row:span 2;"> | ||
| <button class="icon-btn" title="Edit"><svg class="ico"><use href="#i-edit"/></svg></button> | ||
| <button class="icon-btn danger" title="Delete"><svg class="ico"><use href="#i-trash"/></svg></button> | ||
| <label class="switch" title="Enabled"><input type="checkbox" checked><span class="switch-track"><span class="switch-knob"/></span></label> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Replace self-closing <span /> with explicit closing tags.
<span class="switch-knob"/> is invalid HTML style for non-void elements and can produce parser-dependent DOM structure.
Suggested fix
-<span class="switch-track"><span class="switch-knob"/></span>
+<span class="switch-track"><span class="switch-knob"></span></span>Also applies to: 1024-1024, 1047-1047, 1144-1144, 1162-1162, 1180-1180, 1197-1197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design/automations-redesign.html` at line 1000, The switch markup uses
self-closing spans, which is invalid for non-void elements and can lead to
parser-dependent DOM structure. Update the repeated switch templates in the
automation redesign HTML so the switch-knob element uses an explicit opening and
closing span tag, and apply the same fix to every matching instance in the
affected markup.
| <div class="card" data-state="success"> | ||
| <div class="card-top"> | ||
| <div class="card-title-row"> | ||
| <span class="card-name">Triange stale issues</span> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix typo in automation title.
“Triange stale issues” should be “Triage stale issues”.
Also applies to: 1089-1089
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@design/automations-redesign.html` at line 1036, The automation title contains
a typo in the card-name text used for the stale issues workflow. Update the
string in the relevant card markup so “Triange stale issues” becomes “Triage
stale issues”, and make the same correction in the other duplicate occurrence
referenced by the review comment.
| // The run drilled into from the Automations page (clicking a run row, or Run | ||
| // again from a card). Resolved against the automation it belongs to so the | ||
| // detail header can show the name + schedule. | ||
| const activeRun = ref<AutomationRun | null>(null) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the active run from the store instead of storing a stale snapshot.
activeRun keeps the clicked run object, but polling refreshes automationStore.runs by replacement. The detail prop can therefore stay stale after completion, leaving status/duration/error fields outdated.
Proposed direction
-import { ref, onMounted, nextTick, watch, onUnmounted, provide } from 'vue'
+import { ref, computed, onMounted, nextTick, watch, onUnmounted, provide } from 'vue'
@@
-const activeRun = ref<AutomationRun | null>(null)
+const activeRunSnapshot = ref<AutomationRun | null>(null)
+const activeRun = computed(() => {
+ const snapshot = activeRunSnapshot.value
+ if (!snapshot) return null
+ return automationStore.runs.find((r) => r.session_id === snapshot.session_id) ?? snapshot
+})
@@
function openAutomationRun(run: AutomationRun) {
- activeRun.value = run
+ activeRunSnapshot.value = run
activeView.value = 'automation-run'
}Also applies to: 106-113, 677-681
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/App.vue` around lines 50 - 53, The active run detail is holding a
stale run snapshot instead of tracking the refreshed store state. Update the
App.vue flow around activeRun, showRunDetails, and the run detail binding so the
selected run is derived from automationStore.runs (matched by id) whenever the
store refreshes, rather than keeping the originally clicked object. Make sure
the detail header and status/duration/error fields always read from the current
store record, and adjust any related computed/watch logic used in the active run
selection.
| // Mark any tool calls that never got a result as done (replay of a finished run). | ||
| for (const tc of pending.values()) tc.status = 'done' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep unresolved tool calls running during live replays.
applyEntries() also runs for live sessions. Marking unmatched tool_calls as done makes in-flight tools appear completed before their result arrives.
Proposed fix
- // Mark any tool calls that never got a result as done (replay of a finished run).
- for (const tc of pending.values()) tc.status = 'done'
+ // Mark orphaned tool calls complete only when replaying a terminal run.
+ if (!running.value) {
+ for (const tc of pending.values()) tc.status = 'done'
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Mark any tool calls that never got a result as done (replay of a finished run). | |
| for (const tc of pending.values()) tc.status = 'done' | |
| // Mark orphaned tool calls complete only when replaying a terminal run. | |
| if (!running.value) { | |
| for (const tc of pending.values()) tc.status = 'done' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/AutomationRunView.vue` around lines 120 - 121, The replay
cleanup in applyEntries is incorrectly finalizing unmatched tool_call entries,
which breaks live sessions by marking in-flight calls as done too early. Update
AutomationRunView’s applyEntries handling so only finished-run replays close out
unresolved pending tool calls, and leave them active during live sessions until
their result entry arrives. Use the pending collection logic in applyEntries and
the tool_call status update path to scope this behavior correctly.
| async function stopRun() { | ||
| try { | ||
| await api.stop(props.run.session_id) | ||
| running.value = false | ||
| } catch { | ||
| // Surface as a stopped state regardless — the user asked to stop. | ||
| running.value = false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t show a failed stop request as stopped.
If /api/stop fails, the run can still be executing, but the UI immediately flips to a completed-looking state. Surface the error and let polling confirm termination.
Proposed fix
async function stopRun() {
try {
await api.stop(props.run.session_id)
- running.value = false
- } catch {
- // Surface as a stopped state regardless — the user asked to stop.
- running.value = false
+ await store.fetchAll()
+ } catch (e) {
+ error.value = e instanceof Error ? e.message : String(e)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function stopRun() { | |
| try { | |
| await api.stop(props.run.session_id) | |
| running.value = false | |
| } catch { | |
| // Surface as a stopped state regardless — the user asked to stop. | |
| running.value = false | |
| } | |
| async function stopRun() { | |
| try { | |
| await api.stop(props.run.session_id) | |
| await store.fetchAll() | |
| } catch (e) { | |
| error.value = e instanceof Error ? e.message : String(e) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/AutomationRunView.vue` around lines 151 - 158, The
stopRun() handler in AutomationRunView.vue is forcing running.value to false
even when api.stop(props.run.session_id) fails, which makes a failed stop look
like the run stopped successfully. Update stopRun() so the catch path surfaces
the error instead of changing the UI to stopped, and let the existing
polling/state refresh confirm when the run has actually terminated. Keep the
success path in stopRun() unchanged and preserve the run state until the API
call succeeds or polling updates it.
| .status-chip.lg { padding: 4px 9px; font-size: 11.5px; } | ||
| .status-chip.success { color: var(--color-success-fg); background: var(--color-success-bg); } | ||
| .status-chip.error { color: var(--color-error-fg); background: var(--color-error-bg); } | ||
| .status-chip.running { color: var(--color-primary); background: var(--accent-wash, color-mix(in srgb, var(--color-primary) 11%, transparent)); } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move inline color fallbacks into tokens.
These color-mix(... transparent) fallbacks still define colors in the component. Use token-only variables and define the accent tokens in src/styles/tokens.css.
As per coding guidelines, “Every color must come from a CSS custom property defined in src/styles/tokens.css. Never hardcode hex/rgb/#fff/white in .vue or .css.”
Also applies to: 342-351
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/AutomationRunView.vue` at line 315, The inline color
fallback in AutomationRunView’s status-chip styles still hardcodes a color-mix
value instead of relying on design tokens. Remove the component-level fallback
from the .status-chip.running and related status-chip rules, switch them to
token-only custom properties, and add the missing accent token definitions in
src/styles/tokens.css so all colors are sourced from tokens only.
Source: Coding guidelines
What
Adds an Automations feature to jcode — run a task on a schedule (hourly/daily/weekly) or manually. Modeled on Claude Code's Automations UI (list page · new-automation modal · templates), adapted to jcode's architecture. A run is just a normal session tagged with the automation id, so it reuses the existing Engine, transcript, diff, and notifications.
Design doc + full edge-case analysis:
docs/automations-prd.md.How it's built
Core —
internal/automation/(leaf package, no web/tui deps, fully unit-tested):types/validate/schedule(pureComputeNextRun, DST-correct) /templatesautomations.json(definitions) +automation-state.json(volatile scheduler state), separated so the scheduler's frequent writes never clobber human edits. Cross-process flock write lock + atomic temp-rename.recover()so a panicking run can't crash the host process.Web —
internal/web/:automation_run.go: Runner reusesbuildLocalEngine+submitMessageheadlessly, captures the terminal error via an event-handler wrapper, and tears the throwaway engine down on completion (there is no idle-evict). Scheduled runs forced tofull_access(headless approvals would block).automation_api.go: REST CRUD + run-now + runs + templates. PUT is a true partial patch (pointer fields) so editing never wipes a provider/model override or re-enables a paused automation.SessionMetagainsautomation_id / trigger_kind / terminal_status / end_time / error_reason. The main task list excludes automation runs; Recent runs filters them in with a Success/Failed status filter (the audit trail).Agent tool (web/TUI/ACP):
automation_createlets the agent propose an automation from natural language, but it's always created DISABLED (human-in-the-loop) — a prompt-injected agent can't silently arm a recurring unattended run.CLI:
jcode automation list | show | templates | enable | disable | delete.Frontend (Vue): Automations overlay (list + recent runs + templates), New/Edit modal (cloud toggle greyed "coming soon"), Pinia store, sidebar entry, i18n (5 locales).
Testing
internal/automation: schedule (incl. both DST transitions), slot dedup, validation, store round-trip + two-file separation + corrupt-file tolerance, ExecuteRun success/error/auto-disable, panic recovery, scheduler tick (seed→fire / skip-if-running / slot-dedup / missing-project).internal/web: API CRUD + partial-patch preservation + validation + setup-mode + templates (in-processhttptest).go build/vet/test ./...,vue-tsc,vite buildall green.Notes / follow-ups (design in PRD)
automation_createuses disabled-create as the human-in-the-loop gate; the richer in-chat confirmation card is a follow-up.ask_useris currently backstopped by the 30 min ceiling; true toolset exclusion is a follow-up.jcode web(fires while the app is open); ajcode daemonfor "runs when the app is closed" is a documented Phase 2.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes